Skip to content

Conversation

@makslevental
Copy link
Contributor

@makslevental makslevental commented Apr 10, 2025

This PR bumps LLVM to the main commit associated with llvm/llvm-project#131480. The only non-upstreaming-SMT change is the small change in test/Dialect/Moore/types-errors.mlir.

Note, the upstream PR was only for the dialect itself. Shortly I will send subsequent PRs to move all of the other SMT stuff (ExportSMITLib, CAPI, Python bindings).

@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch 4 times, most recently from 3a81a31 to 05a2409 Compare April 11, 2025 21:16
@makslevental makslevental marked this pull request as ready for review April 11, 2025 21:19
@makslevental makslevental requested a review from maerhart as a code owner April 11, 2025 21:19
@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from 05a2409 to 3b4189b Compare April 12, 2025 19:47
Copy link
Contributor

@TaoBi22 TaoBi22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks!

ConversionPatternRewriter &rewriter) const override {
rewriter.replaceOpWithNewOp<smt::RepeatOp>(op, op.getMultiple(),
adaptor.getInput());
rewriter.replaceOpWithNewOp<mlir::smt::RepeatOp>(op, op.getMultiple(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not just using namespace mlir in this file?

Copy link
Contributor Author

@makslevental makslevental Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I did that but then I couldn't tell what convention you guys are following - whether code outside of CIRCT should be fully qualified. If you say using namespace mlir is ok that's good enough for me!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely not an authority on our style conventions, but it looks like plenty of conversion passes use it!

Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for doing the integration!

@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from e75ef0e to c930fe2 Compare April 14, 2025 17:45
@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from c930fe2 to e32b67b Compare April 14, 2025 18:25
@makslevental makslevental merged commit 7ea4014 into llvm:main Apr 14, 2025
@makslevental makslevental deleted the makslevental/integrate-upstream-smt branch April 14, 2025 18:34
RonxBulld pushed a commit to RonxBulld/hs-circt that referenced this pull request Apr 15, 2025
KelvinChung2000 pushed a commit to KelvinChung2000/circt that referenced this pull request Apr 22, 2025
TaoBi22 pushed a commit to TaoBi22/circt that referenced this pull request Jul 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants